Skip to content

SONARJAVA-6686 Implement new rule S2198: Unnecessary mathematical comparisons - #5845

Draft
romainbrenguier wants to merge 5 commits into
masterfrom
new-rule/SONARJAVA-6686-S2198
Draft

SONARJAVA-6686 Implement new rule S2198: Unnecessary mathematical comparisons#5845
romainbrenguier wants to merge 5 commits into
masterfrom
new-rule/SONARJAVA-6686-S2198

Conversation

@romainbrenguier

Copy link
Copy Markdown
Contributor

Detect comparisons between a bounded numeric primitive (byte, short, char, int) and a compile-time constant outside the type's value range, which always evaluate to true or false.

Part of

@hashicorp-vault-sonar-prod

hashicorp-vault-sonar-prod Bot commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

SONARJAVA-6686

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like it's was not addded to the profiles

@github-actions

Copy link
Copy Markdown
Contributor

Ruling needs updating. A fix PR has been created: #5846

Please review and merge it into your branch.

@github-actions

Copy link
Copy Markdown
Contributor

Ruling needs updating. A fix PR has been created: #5846

Please review and merge it into your branch.

@github-actions

github-actions Bot commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

Ruling Diff Summary

Detected changes in 3 rule files: 0 issues removed, 12 issues added.

S2198 (java) on eclipse-jetty - 0 issues removed, 9 issues added - new ruling file

Added jetty-http/src/main/java/org/eclipse/jetty/http/HttpGenerator.java (line 876)

(source file not found at this revision: jetty-http/src/main/java/org/eclipse/jetty/http/HttpGenerator.java)

Added jetty-http/src/main/java/org/eclipse/jetty/http/HttpGenerator.java (line 890)

(source file not found at this revision: jetty-http/src/main/java/org/eclipse/jetty/http/HttpGenerator.java)

Added jetty-util/src/main/java/org/eclipse/jetty/util/ByteArrayISO8859Writer.java (line 107)

(source file not found at this revision: jetty-util/src/main/java/org/eclipse/jetty/util/ByteArrayISO8859Writer.java)

Added jetty-util/src/main/java/org/eclipse/jetty/util/ByteArrayISO8859Writer.java (line 124)

(source file not found at this revision: jetty-util/src/main/java/org/eclipse/jetty/util/ByteArrayISO8859Writer.java)

Added jetty-util/src/main/java/org/eclipse/jetty/util/ByteArrayISO8859Writer.java (line 142)

(source file not found at this revision: jetty-util/src/main/java/org/eclipse/jetty/util/ByteArrayISO8859Writer.java)

Added jetty-util/src/main/java/org/eclipse/jetty/util/TreeTrie.java (line 98)

(source file not found at this revision: jetty-util/src/main/java/org/eclipse/jetty/util/TreeTrie.java)

Added jetty-util/src/main/java/org/eclipse/jetty/util/TreeTrie.java (line 135)

(source file not found at this revision: jetty-util/src/main/java/org/eclipse/jetty/util/TreeTrie.java)
S2198 (java) on eclipse-jetty-similar-to-main - 0 issues removed, 2 issues added - new ruling file

Added jetty-http/src/main/java/org/eclipse/jetty/http/HttpGenerator.java (line 876)

(source file not found at this revision: jetty-http/src/main/java/org/eclipse/jetty/http/HttpGenerator.java)

Added jetty-http/src/main/java/org/eclipse/jetty/http/HttpGenerator.java (line 890)

(source file not found at this revision: jetty-http/src/main/java/org/eclipse/jetty/http/HttpGenerator.java)
S2198 (java) on guava - 0 issues removed, 1 issues added - new ruling file

Added src/com/google/common/net/InetAddresses.java (line 907)

       902 |     // Squash into 224/4 Multicast and 240/4 Reserved space (i.e. 224/3).
       903 |     coercedHash |= 0xe0000000;
       904 | 
       905 |     // Fixup to avoid some "illegal" values.  Currently the only potential
       906 |     // illegal value is 255.255.255.255.
>>>    907 |     if (coercedHash == 0xffffffff) {
       908 |       coercedHash = 0xfffffffe;
       909 |     }
       910 | 
       911 |     return getInet4Address(Ints.toByteArray(coercedHash));
       912 |   }

@github-actions

Copy link
Copy Markdown
Contributor

Ruling needs updating. A fix PR has been created: #5846

Please review and merge it into your branch.

romainbrenguier pushed a commit that referenced this pull request Aug 3, 2026
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
@github-actions

github-actions Bot commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

Ruling needs updating. A fix PR has been created: #5856

Please review and merge it into your branch.

romainbrenguier pushed a commit that referenced this pull request Aug 3, 2026
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
@romainbrenguier
romainbrenguier force-pushed the new-rule/SONARJAVA-6686-S2198 branch from 1212538 to 691c064 Compare August 3, 2026 08:16
romainbrenguier and others added 4 commits August 3, 2026 10:27
Detect comparisons between a bounded numeric primitive (byte, short, char,
int) and a compile-time constant outside the type's value range, which
always evaluate to true or false.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Move trailing comment to previous empty line (convention)
- Add missing curly braces on if/else statements
- Reduce cognitive complexity by extracting helper methods

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
@romainbrenguier
romainbrenguier force-pushed the new-rule/SONARJAVA-6686-S2198 branch from 691c064 to a1ce778 Compare August 3, 2026 08:27
@github-actions

github-actions Bot commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

Ruling needs updating. A fix PR has been created: #5856

Please review and merge it into your branch.

1 similar comment
@github-actions

github-actions Bot commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

Ruling needs updating. A fix PR has been created: #5856

Please review and merge it into your branch.

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
@gitar-bot

gitar-bot Bot commented Aug 3, 2026

Copy link
Copy Markdown
CI failed: Integration test failures in JavaRulingTest caused by missing or outdated ruling expectations for the newly introduced rule S2198.

Overview

All analyzed CI failures stem from test assertions in JavaRulingTest failing during integration/ruling QA runs (such as eclipse_jetty_incremental and guava). These failures occur because the newly introduced rule S2198 ("Unnecessary mathematical comparisons") generates new issues that do not match the existing expected ruling test baselines.

Failures

Ruling Integration Test Mismatches (confidence: high)

  • Type: test
  • Affected jobs: 91612629031, 90297643213
  • Related to change: yes
  • Root cause: The new rule S2198 produces new analysis issues on tested open-source projects (e.g., Eclipse Jetty, Guava), causing comparison assertions between expected and actual issues in JavaRulingTest to fail.
  • Suggested fix: Update the expected ruling test baselines and JSON result files under its/ruling/src/test/resources/ to include the expected issues raised by rule S2198.

Summary

  • Change-related failures: 2 test failures in integration ruling tests due to new issues from rule S2198.
  • Infrastructure/flaky failures: 0
  • Recommended action: Update the ruling test expectations/baselines to incorporate the output of the new S2198 rule.
Code Review ✅ Approved 2 resolved / 2 findings

Implements new rule S2198 to detect unnecessary mathematical comparisons with out-of-range constants, addressing the rule activation and message stability findings.

✅ 2 resolved
Quality: New rule S2198 may not be activated in Sonar way profile

📄 sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S2198.json:1-15
The rule metadata is marked "status": "ready", but new rules are only enabled by default if their key is added to the Sonar way profile definition (the built-in profile is populated from a JSON key list via BuiltInJavaQualityProfile/GeneratedCheckList, not from rule status). This diff adds the check class and S2198.json but no profile entry, so unless activation is handled elsewhere the rule will ship deactivated by default. Verify whether S2198 should be added to the Sonar way profile rule list.

Quality: Draft PR: sample uses Noncompliant without message stability check

📄 java-checks-test-sources/default/src/main/java/checks/UselessMathematicalComparisonCheckSample.java:77-85 📄 java-checks/src/main/java/org/sonar/java/checks/UselessMathematicalComparisonCheck.java:114-122
The test relies on exact issue messages ("...always return true/false.") embedded in the sample file. This is fine, but note the reversed-operand equality cases (e.g. 200 == b) exercise the symmetric-operator path in reverseOperator; ensure coverage also includes a reversed NOT_EQUAL_TO returning false and reversed EQUAL_TO within-range compliant case to lock in the symmetric handling. Currently no compliant reversed-operand case is asserted, leaving a small gap in regression protection.

Tip

Comment Gitar fix CI or enable auto-apply: gitar auto-apply:on

Options

Auto-apply is off → Gitar will not commit updates to this branch.
Display: compact → Showing less information.

Comment with these commands to change the behavior for this request:

Auto-apply Compact
gitar auto-apply:on         
gitar display:verbose         

Was this helpful? React with 👍 / 👎 | Gitar

@sonarqube-next

sonarqube-next Bot commented Aug 3, 2026

Copy link
Copy Markdown

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant